Assertions#67
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #67 +/- ##
==========================================
- Coverage 92.29% 91.67% -0.62%
==========================================
Files 15 17 +2
Lines 662 829 +167
Branches 44 60 +16
==========================================
+ Hits 611 760 +149
- Misses 46 57 +11
- Partials 5 12 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tgubler9
changed the title
|
Failing because tests of |
calebsyring
force-pushed
the
tim/assertion
branch
from
ee0ccde to
f5aea44
Compare
|
Should be good, but I need to add more tests |
calebsyring
left a comment
calebsyring
left a comment
There was a problem hiding this comment.
Looks good! I have a few minor / nonstructural things to take a look at
| actual = response.text | ||
| elif self.assertion_object == AssertionSubject.RESPONSE_TIME: | ||
| actual = response.elapsed.total_seconds() * 1000 | ||
|
|
There was a problem hiding this comment.
maybe add an else here with an error for unhandled cases
There was a problem hiding this comment.
Shouldnt this be dealt with by the cast function?
There was a problem hiding this comment.
Yes, but if we update the cast function and forget to update this function we might end up with an odd error. Not a big deal either way.
| Makes the request and returns the response and the time it took to make the request. | ||
| """ | ||
|
|
||
| # TODO figure out if we want to use time.perf_counter or built in httpx response latency |
There was a problem hiding this comment.
I think we probably do want to use time.perf_counter so that we still have a latency value available if there is a timeout exception.
| slug='my-monitor', | ||
| url='https://example.com/health', | ||
| ) | ||
| # Pretend we've received data via the API |
There was a problem hiding this comment.
Why did you decide to stop using the factories here?
There was a problem hiding this comment.
This method still needs to be switched back to the factory I think.
There was a problem hiding this comment.
Oop, switched a different one
| # Manually set the elapsed time to 500ms | ||
| resp.elapsed = timedelta(milliseconds=20.1) |
There was a problem hiding this comment.
Comment doesn't match elapsed. Also, this function isn't used anywhere
There was a problem hiding this comment.
Im not sure what you mean its not used anywhere, elapsed is used for the response time in the evaluate method.
There was a problem hiding this comment.
I mean custom_response isn't called anywhere as far as I can tell.
There was a problem hiding this comment.
The ``httpx_mock` is not being used you are correct, so I can just get rid of that. The resp is still being used, and it worked without that fixture as far as I know
3 participants
Implementation of #64
Goals
<Assertion Subject> <operator> <valid expected data>run_checksshould give a reason why the assertion failed